Move redirect rendering into WikitextContent
authorBrad Jorsch <bjorsch@wikimedia.org>
Sat, 4 Jan 2014 22:07:33 +0000 (17:07 -0500)
committerBrad Jorsch <bjorsch@wikimedia.org>
Tue, 7 Jan 2014 14:18:22 +0000 (09:18 -0500)
There's no good reason for everything that wants to render a page to
have to test whether the page is a redirect and then call
Article::viewRedirect to get the fancy rendering instead of using the
ParserOutput. This logic can easily be moved into
WikitextContent::getParserOutput so callers can just use the
returned ParserOutput.

At the same time, we can handle "#REDIRECT [[File:Foo]]" and
"#REDIRECT [[Category:Foo]]" the way people expect, by recording the
link in pagelinks rather than imagelinks/categorylinks (although this
means fixing ImagePage's bug about assuming anything in imagelinks that
is a redirect is a redirect to the image too).

And we can finally fix bug 14323, too.

Bug: 14323
Bug: 17259
Bug: 27621
Bug: 42642
Bug: 50488
Change-Id: Id44d566a7ca35a1b9579d0c0e947877c980b0686
Followup: I1c7582d1bf7ec4184a45b00154e3dd5b39dd444b
Followup: I3653b608941813a73281f4f0545bea2487d43964

RELEASE-NOTES-1.23
includes/Article.php
includes/EditPage.php
includes/ImagePage.php
includes/content/WikitextContent.php
includes/diff/DifferenceEngine.php

index afa7efd..d77a270 100644 (file)
@@ -69,6 +69,9 @@ production.
   'user' or 'bot'. The API will throw an error if the user is not logged
   in (user) or does not have the 'bot' userright (bot). Based off of the
   AssertEdit extension by Steve Sanbeg.
+* WikitextContent will now render redirects with the expected "redirect"
+  header, rather than as an ordered list. Code calling Article::viewRedirect
+  can probably be changed to no longer special-case redirects.
 
 === Bug fixes in 1.23 ===
 * (bug 41759) The "updated since last visit" markers (on history pages, recent
@@ -93,6 +96,8 @@ production.
   the JavaScript evaluator were updated to support the new format. Plural rules
   for some languages have changed, most notably Russian. Affected software
   messages have been updated and marked for review at translatewiki.net.
+* (bug 14323) Redirect pages, when viewed with redirect=no, no longer hide the
+  remaining page content.
 
 === Web API changes in 1.23 ===
 * (bug 54884) action=parse&prop=categories now indicates hidden and missing
index c82b39f..75906fd 100644 (file)
@@ -684,26 +684,15 @@ class Article implements Page {
 
                                                # Allow extensions do their own custom view for certain pages
                                                $outputDone = true;
-                                       } else {
-                                               $content = $this->getContentObject();
-                                               $rt = $content ? $content->getRedirectChain() : null;
-                                               if ( $rt ) {
-                                                       wfDebug( __METHOD__ . ": showing redirect=no page\n" );
-                                                       # Viewing a redirect page (e.g. with parameter redirect=no)
-                                                       $outputPage->addHTML( $this->viewRedirect( $rt ) );
-                                                       # Parse just to get categories, displaytitle, etc.
-                                                       $this->mParserOutput = $content->getParserOutput( $this->getTitle(), $oldid, $parserOptions, false );
-                                                       $outputPage->addParserOutputNoText( $this->mParserOutput );
-                                                       $outputDone = true;
-                                               }
                                        }
                                        break;
                                case 4:
                                        # Run the parse, protected by a pool counter
                                        wfDebug( __METHOD__ . ": doing uncached parse\n" );
 
+                                       $content = $this->getContentObject();
                                        $poolArticleView = new PoolWorkArticleView( $this->getPage(), $parserOptions,
-                                               $this->getRevIdFetched(), $useParserCache, $this->getContentObject() );
+                                               $this->getRevIdFetched(), $useParserCache, $content );
 
                                        if ( !$poolArticleView->execute() ) {
                                                $error = $poolArticleView->getError();
@@ -722,6 +711,9 @@ class Article implements Page {
 
                                        $this->mParserOutput = $poolArticleView->getParserOutput();
                                        $outputPage->addParserOutput( $this->mParserOutput );
+                                       if ( $content->getRedirectTarget() ) {
+                                               $outputPage->addSubtitle( wfMessage( 'redirectpagesub' )->parse() );
+                                       }
 
                                        # Don't cache a dirty ParserOutput object
                                        if ( $poolArticleView->getIsDirty() ) {
@@ -1420,7 +1412,10 @@ class Article implements Page {
        }
 
        /**
-        * View redirect
+        * Return the HTML for the top of a redirect page
+        *
+        * Chances are you should just be using the ParserOutput from
+        * WikitextContent::getParserOutput instead of calling this for redirects.
         *
         * @param $target Title|Array of destination(s) to redirect
         * @param $appendSubtitle Boolean [optional]
@@ -1428,20 +1423,35 @@ class Article implements Page {
         * @return string containing HMTL with redirect link
         */
        public function viewRedirect( $target, $appendSubtitle = true, $forceKnown = false ) {
+               $lang = $this->getTitle()->getPageLanguage();
+               if ( $appendSubtitle ) {
+                       $out = $this->getContext()->getOutput();
+                       $out->addSubtitle( wfMessage( 'redirectpagesub' )->parse() );
+               }
+               return static::getRedirectHeaderHtml( $lang, $target, $forceKnown );
+       }
+
+       /**
+        * Return the HTML for the top of a redirect page
+        *
+        * Chances are you should just be using the ParserOutput from
+        * WikitextContent::getParserOutput instead of calling this for redirects.
+        *
+        * @since 1.23
+        * @param Language $lang
+        * @param Title|array $target destination(s) to redirect
+        * @param bool $forceKnown Should the image be shown as a bluelink regardless of existence?
+        * @return string containing HMTL with redirect link
+        */
+       public static function getRedirectHeaderHtml( Language $lang, $target, $forceKnown = false ) {
                global $wgStylePath;
 
                if ( !is_array( $target ) ) {
                        $target = array( $target );
                }
 
-               $lang = $this->getTitle()->getPageLanguage();
                $imageDir = $lang->getDir();
 
-               if ( $appendSubtitle ) {
-                       $out = $this->getContext()->getOutput();
-                       $out->addSubtitle( wfMessage( 'redirectpagesub' )->parse() );
-               }
-
                // the loop prepends the arrow image before the link, so the first case needs to be outside
 
                /**
index ba49378..1a69707 100644 (file)
@@ -3222,36 +3222,30 @@ HTML
                                }
                        }
 
-                       $rt = $content->getRedirectChain();
-                       if ( $rt ) {
-                               $previewHTML = $this->mArticle->viewRedirect( $rt, false );
-                       } else {
-
-                               # If we're adding a comment, we need to show the
-                               # summary as the headline
-                               if ( $this->section === "new" && $this->summary !== "" ) {
-                                       $content = $content->addSectionHeader( $this->summary );
-                               }
+                       # If we're adding a comment, we need to show the
+                       # summary as the headline
+                       if ( $this->section === "new" && $this->summary !== "" ) {
+                               $content = $content->addSectionHeader( $this->summary );
+                       }
 
-                               $hook_args = array( $this, &$content );
-                               ContentHandler::runLegacyHooks( 'EditPageGetPreviewText', $hook_args );
-                               wfRunHooks( 'EditPageGetPreviewContent', $hook_args );
+                       $hook_args = array( $this, &$content );
+                       ContentHandler::runLegacyHooks( 'EditPageGetPreviewText', $hook_args );
+                       wfRunHooks( 'EditPageGetPreviewContent', $hook_args );
 
-                               $parserOptions->enableLimitReport();
+                       $parserOptions->enableLimitReport();
 
-                               # For CSS/JS pages, we should have called the ShowRawCssJs hook here.
-                               # But it's now deprecated, so never mind
+                       # For CSS/JS pages, we should have called the ShowRawCssJs hook here.
+                       # But it's now deprecated, so never mind
 
-                               $content = $content->preSaveTransform( $this->mTitle, $wgUser, $parserOptions );
-                               $parserOutput = $content->getParserOutput( $this->getArticle()->getTitle(), null, $parserOptions );
+                       $content = $content->preSaveTransform( $this->mTitle, $wgUser, $parserOptions );
+                       $parserOutput = $content->getParserOutput( $this->getArticle()->getTitle(), null, $parserOptions );
 
-                               $previewHTML = $parserOutput->getText();
-                               $this->mParserOutput = $parserOutput;
-                               $wgOut->addParserOutputNoText( $parserOutput );
+                       $previewHTML = $parserOutput->getText();
+                       $this->mParserOutput = $parserOutput;
+                       $wgOut->addParserOutputNoText( $parserOutput );
 
-                               if ( count( $parserOutput->getWarnings() ) ) {
-                                       $note .= "\n\n" . implode( "\n\n", $parserOutput->getWarnings() );
-                               }
+                       if ( count( $parserOutput->getWarnings() ) ) {
+                               $note .= "\n\n" . implode( "\n\n", $parserOutput->getWarnings() );
                        }
                } catch ( MWContentSerializationException $ex ) {
                        $m = wfMessage( 'content-failed-to-parse', $this->contentModel, $this->contentFormat, $ex->getMessage() );
index 997a948..a83217d 100644 (file)
@@ -732,7 +732,7 @@ EOT
 
                return $dbr->select(
                        array( 'imagelinks', 'page' ),
-                       array( 'page_namespace', 'page_title', 'page_is_redirect', 'il_to' ),
+                       array( 'page_namespace', 'page_title', 'il_to' ),
                        array( 'il_to' => $target, 'il_from = page_id' ),
                        __METHOD__,
                        array( 'LIMIT' => $limit + 1, 'ORDER BY' => 'il_from', )
@@ -743,13 +743,19 @@ EOT
                $limit = 100;
 
                $out = $this->getContext()->getOutput();
-               $res = $this->queryImageLinks( $this->getTitle()->getDBkey(), $limit + 1 );
+
                $rows = array();
                $redirects = array();
+               foreach ( $this->getTitle()->getRedirectsHere( NS_FILE ) as $redir ) {
+                       $redirects[$redir->getDBkey()] = array();
+                       $rows[] = (object)array(
+                               'page_namespace' => NS_FILE,
+                               'page_title' => $redir->getDBkey(),
+                       );
+               }
+
+               $res = $this->queryImageLinks( $this->getTitle()->getDBkey(), $limit + 1 );
                foreach ( $res as $row ) {
-                       if ( $row->page_is_redirect ) {
-                               $redirects[$row->page_title] = array();
-                       }
                        $rows[] = $row;
                }
                $count = count( $rows );
index 1f96bdc..a92699b 100644 (file)
@@ -152,29 +152,27 @@ class WikitextContent extends TextContent {
        }
 
        /**
-        * Implement redirect extraction for wikitext.
-        *
-        * @return null|Title
+        * Extract the redirect target and the remaining text on the page.
         *
         * @note: migrated here from Title::newFromRedirectInternal()
         *
-        * @see Content::getRedirectTarget
-        * @see AbstractContent::getRedirectTarget
+        * @since 1.23
+        * @return array 2 elements: Title|null and string
         */
-       public function getRedirectTarget() {
+       protected function getRedirectTargetAndText() {
                global $wgMaxRedirects;
                if ( $wgMaxRedirects < 1 ) {
                        // redirects are disabled, so quit early
-                       return null;
+                       return array( null, $this->getNativeData() );
                }
                $redir = MagicWord::get( 'redirect' );
-               $text = trim( $this->getNativeData() );
+               $text = ltrim( $this->getNativeData() );
                if ( $redir->matchStartAndRemove( $text ) ) {
                        // Extract the first link and see if it's usable
                        // Ensure that it really does come directly after #REDIRECT
                        // Some older redirects included a colon, so don't freak about that!
                        $m = array();
-                       if ( preg_match( '!^\s*:?\s*\[{2}(.*?)(?:\|.*?)?\]{2}!', $text, $m ) ) {
+                       if ( preg_match( '!^\s*:?\s*\[{2}(.*?)(?:\|.*?)?\]{2}\s*!', $text, $m ) ) {
                                // Strip preceding colon used to "escape" categories, etc.
                                // and URL-decode links
                                if ( strpos( $m[1], '%' ) !== false ) {
@@ -184,14 +182,27 @@ class WikitextContent extends TextContent {
                                $title = Title::newFromText( $m[1] );
                                // If the title is a redirect to bad special pages or is invalid, return null
                                if ( !$title instanceof Title || !$title->isValidRedirectTarget() ) {
-                                       return null;
+                                       return array( null, $this->getNativeData() );
                                }
 
-                               return $title;
+                               return array( $title, substr( $text, strlen( $m[0] ) ) );
                        }
                }
 
-               return null;
+               return array( null, $this->getNativeData() );
+       }
+
+       /**
+        * Implement redirect extraction for wikitext.
+        *
+        * @return null|Title
+        *
+        * @see Content::getRedirectTarget
+        * @see AbstractContent::getRedirectTarget
+        */
+       public function getRedirectTarget() {
+               list( $title, ) = $this->getRedirectTargetAndText();
+               return $title;
        }
 
        /**
@@ -303,7 +314,21 @@ class WikitextContent extends TextContent {
                        $options = $this->getContentHandler()->makeParserOptions( 'canonical' );
                }
 
-               $po = $wgParser->parse( $this->getNativeData(), $title, $options, true, true, $revId );
+               list( $redir, $text ) = $this->getRedirectTargetAndText();
+               $po = $wgParser->parse( $text, $title, $options, true, true, $revId );
+
+               // Add redirect indicator at the top
+               if ( $redir ) {
+                       // Make sure to include the redirect link in pagelinks
+                       $po->addLink( $redir );
+                       if ( $generateHtml ) {
+                               $chain = $this->getRedirectChain();
+                               $po->setText(
+                                       Article::getRedirectHeaderHtml( $title->getPageLanguage(), $chain, false ) .
+                                       $po->getText()
+                               );
+                       }
+               }
 
                return $po;
        }
index 6e74f2c..b7601e9 100644 (file)
@@ -580,19 +580,8 @@ class DifferenceEngine extends ContextSource {
 
                                $parserOutput = $this->getParserOutput( $wikiPage, $this->mNewRev );
 
-                               # Also try to load it as a redirect
-                               $rt = $this->mNewContent ? $this->mNewContent->getRedirectTarget() : null;
-
-                               if ( $rt ) {
-                                       $article = Article::newFromTitle( $this->mNewPage, $this->getContext() );
-                                       $out->addHTML( $article->viewRedirect( $rt ) );
-
-                                       # WikiPage::getParserOutput() should not return false, but just in case
-                                       if ( $parserOutput ) {
-                                               # Show categories etc.
-                                               $out->addParserOutputNoText( $parserOutput );
-                                       }
-                               } elseif ( $parserOutput ) {
+                               # WikiPage::getParserOutput() should not return false, but just in case
+                               if ( $parserOutput ) {
                                        $out->addParserOutput( $parserOutput );
                                }
                        }